Repository navigation
Share measured text layouts across unchanged Text clones instead of copying them - #59357
Closed
hryhoriiK97 wants to merge 1 commit into
Closed
hryhoriiK97 wants to merge 1 commit into
hryhoriiK97 wants to merge 1 commit into
Conversation
…opying them Since react#59285, a ParagraphShadowNode cloned without new props or children keeps the layout its source used. It kept it by copying the MeasuredPreparedTextLayout, and on Android that struct holds a JNI global reference, so every such clone called NewGlobalRef (and later DeleteGlobalRef). Layouts are never modified once measured, so store them as shared_ptr<const MeasuredPreparedTextLayout> and let clones share them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
javache
approved these changes
Oct 23, 2026
|
@javache has imported this pull request. If you are a Meta employee, you can view this in D127127541. |
Collaborator
|
This pull request was successfully merged by @hryhoriiK97 in 0e041dd When will my fix make it into a release? | How to file a pick request? |
usama-liaqat
pushed a commit
to usama-liaqat/react-native
that referenced
this pull request
Nov 14, 2026
…opying them (react#59357) Summary: Since react#59285, a `ParagraphShadowNode` cloned without new props or children (for example by Yoga, because a sibling changed) keeps the measured layout its source used, so it doesn't have to measure again. It keeps it by copying `measuredLayouts_.back()`, a `MeasuredPreparedTextLayout`. **Current behavior.** On Android, `MeasuredPreparedTextLayout::preparedTextLayout` is a `SafeReleaseJniRef<jni::global_ref<JPreparedLayout>>`, whose copy constructor calls JNI `NewGlobalRef`, and the copy later releases it with `DeleteGlobalRef`. Every unchanged Text clone therefore creates and deletes a JNI global reference on the JS thread. Scrolling a text-heavy list does this about 2,500 times a second. In a Perfetto profile of RNTester on a Pixel 9a, `NewGlobalRef` from `ParagraphShadowNode`'s clone constructor was 2.3% of all JS-thread time (2.9% for all `NewGlobalRef` calls on that thread). **With this change.** A layout is never modified once it is measured, so `measuredLayouts_` holds `std::shared_ptr<const MeasuredPreparedTextLayout>`, and a clone shares the layout instead of copying it. The rest of the logic is unchanged: - The clone constructor keeps the same layout under the same conditions. - `measureContent` and `findUsableLayout` look layouts up in the same order with the same checks (`findUsableLayout` now returns a `const` pointer). - `layout()` still keeps only the layout it used, now with `std::erase_if` on the pointers. Before, it moved the used layout out of the vector, which sharing would no longer allow. The prepared Java layout object was already shared before: copying a `global_ref` adds a reference to the same object. Its lifetime is unchanged, and `SafeReleaseJniRef` attaches the thread before releasing, so releasing the last reference on any thread is safe. Only Android implements prepared text layouts; on other platforms `PreparedTextLayout` is `std::nullptr_t` and the code that uses it doesn't run. ## Changelog: [ANDROID] [CHANGED] - Text nodes cloned without changes share their measured layout instead of creating a new JNI global reference Pull Request resolved: react#59357 Test Plan: - `scripts/clang-format.js --check` passes on both files. - **Device:** Pixel 9a, Android 16, RNTester built with Gradle in Release (`assembleRelease`, arm64-v8a) and compiled with `cmd package compile -m speed` before each run. The scenarios are local Playground screens that are not part of this PR: - a 1,000-row feed with posts (avatar, name, wrapped text, action row), headers, horizontal carousels and cards that scrolls itself with `scrollToOffset` on a timer, so no touch automation runs during measurements; - a screen stepped with a button through the live changes listed in react#59285 test plan. - **A/B setup:** a single binary with a local switch (an `adb shell setprop` read at startup, not part of this PR) that makes the clone constructor either share the layout (this PR) or copy it, as the code did before. Both arms run the same install. - **Cost per kept layout**, measured in the clone constructor, over about 127,000 kept layouts per launch, in 4 to 6 launches per arm: | | copy (before) | share (this PR) | change | |---|--:|--:|--:| | Time to keep a layout | 4.1–4.5 µs | 0.80–0.86 µs | −81% | - **JS-thread CPU** while the feed scrolls, read from `/proc/<pid>/task/*/stat` over the same 40 s window of each launch, launches alternating between the arms: | Series | copy (before) | share (this PR) | change | |---|--:|--:|--:| | 1 (6 + 4 launches) | 12,498 ms | 12,165 ms | −2.7% | | 2, 60 Hz fixed, reverse order (6 + 3 launches) | 12,632 ms | 12,280 ms | −2.8% | This matches the time saved in the clone constructor (about 3.5 µs × 127,000 ≈ 0.45 s per launch, of which about 0.36 s falls in the 40 s window). The main thread and RenderThread didn't change. This feed sometimes settles into rendering a larger window of cells, which makes about 25% more clones for the whole launch and raises JS-thread CPU by about 1 s. It happened with the copy arm too (1 of 21 launches), and more often with the share arm (9 of 21). With the share arm slowed down by a 3.5 µs spin to match the copy's cost, it didn't happen in 4 launches. So it depends on how much time the JS thread has, not on what this change does. The series above compare launches with the same amount of work (126,000–130,000 kept layouts). - **Perfetto** (Argent's native profiler, 40 s of scrolling): `NewGlobalRef` samples under `ParagraphShadowNode`'s clone constructor went from 31 to 0, and all `NewGlobalRef` samples on the JS thread from 2.9% to 0.2% of the thread's samples. - **Rendering**, the same screens captured in both arms and compared pixel by pixel with Argent's `screenshot-diff`. Every state matches. A second copy-arm pass shows the same 0.20% at the first step, so that difference is capture noise. The states differ from each other (for example 4.5% between the first and fourth step), so a match means the screen was compared after it changed. | live change (as in react#59285 test plan) | copy vs share | |---|---| | update one sibling's text (the cloned-without-changes path) | identical | | `adjustsFontSizeToFit` text in a container of height 20 → 40 → 60 → 20 → 40 | identical | | `direction` RTL → LTR on the parent | identical | | font scale 1.0 → 1.3 → 1.0 while the app runs | identical | - The exact code of this PR, without the switch, built and run on the same Pixel: the feed renders and scrolls, and there were no crashes. - **Low-end device:** Samsung Galaxy A14 (SM-A145R, Android 15), the same build and setup, launches alternating A B B A A B B A A B B A (one launch dropped: its CPU reading was invalid). Every launch kept between 80,000 and 100,000 layouts. | | copy (before) | share (this PR) | change | |---|--:|--:|--:| | Time to keep a layout | 4.8–5.5 µs | 0.60–0.71 µs | −87% | | JS-thread CPU over the same 40 s window (6 + 5 launches) | 17,742 ms | 17,214 ms | −3.0% (median −2.2%) | The main thread didn't change. Perfetto: `NewGlobalRef` samples under `ParagraphShadowNode`'s clone constructor went from 201 to 0, and all `NewGlobalRef` samples on the JS thread from 1.9% to 0.1% of the thread's samples. - **Not run:** the pixel-density change case (`fixTextLayoutOnPixelDensityChange`), the `com.facebook.react.views.text` unit tests (this PR doesn't touch Java), Fantom (its text layout doesn't implement prepared layouts, so it doesn't reach this code), and an iOS build. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: cipolleschi Differential Revision: D127127541 Pulled By: javache fbshipit-source-id: 011a4faac3a1097e356b875b525f187f115dd9ac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Since #59285, a
ParagraphShadowNodecloned without new props or children (for example by Yoga, because a sibling changed) keeps the measured layout its source used, so it doesn't have to measure again. It keeps it by copyingmeasuredLayouts_.back(), aMeasuredPreparedTextLayout.Current behavior. On Android,
MeasuredPreparedTextLayout::preparedTextLayoutis aSafeReleaseJniRef<jni::global_ref<JPreparedLayout>>, whose copy constructor calls JNINewGlobalRef, and the copy later releases it withDeleteGlobalRef. Every unchanged Text clone therefore creates and deletes a JNI global reference on the JS thread. Scrolling a text-heavy list does this about 2,500 times a second. In a Perfetto profile of RNTester on a Pixel 9a,NewGlobalReffromParagraphShadowNode's clone constructor was 2.3% of all JS-thread time (2.9% for allNewGlobalRefcalls on that thread).With this change. A layout is never modified once it is measured, so
measuredLayouts_holdsstd::shared_ptr<const MeasuredPreparedTextLayout>, and a clone shares the layout instead of copying it. The rest of the logic is unchanged:measureContentandfindUsableLayoutlook layouts up in the same order with the same checks (findUsableLayoutnow returns aconstpointer).layout()still keeps only the layout it used, now withstd::erase_ifon the pointers. Before, it moved the used layout out of the vector, which sharing would no longer allow.The prepared Java layout object was already shared before: copying a
global_refadds a reference to the same object. Its lifetime is unchanged, andSafeReleaseJniRefattaches the thread before releasing, so releasing the last reference on any thread is safe. Only Android implements prepared text layouts; on other platformsPreparedTextLayoutisstd::nullptr_tand the code that uses it doesn't run.Changelog:
[ANDROID] [CHANGED] - Text nodes cloned without changes share their measured layout instead of creating a new JNI global reference
Test Plan:
scripts/clang-format.js --checkpasses on both files.assembleRelease, arm64-v8a) and compiled withcmd package compile -m speedbefore each run. The scenarios are local Playground screens that are not part of this PR:scrollToOffseton a timer, so no touch automation runs during measurements;adb shell setpropread at startup, not part of this PR) that makes the clone constructor either share the layout (this PR) or copy it, as the code did before. Both arms run the same install./proc/<pid>/task/*/statover the same 40 s window of each launch, launches alternating between the arms:This matches the time saved in the clone constructor (about 3.5 µs × 127,000 ≈ 0.45 s per launch, of which about 0.36 s falls in the 40 s window). The main thread and RenderThread didn't change.
This feed sometimes settles into rendering a larger window of cells, which makes about 25% more clones for the whole launch and raises JS-thread CPU by about 1 s. It happened with the copy arm too (1 of 21 launches), and more often with the share arm (9 of 21). With the share arm slowed down by a 3.5 µs spin to match the copy's cost, it didn't happen in 4 launches. So it depends on how much time the JS thread has, not on what this change does. The series above compare launches with the same amount of work (126,000–130,000 kept layouts).
NewGlobalRefsamples underParagraphShadowNode's clone constructor went from 31 to 0, and allNewGlobalRefsamples on the JS thread from 2.9% to 0.2% of the thread's samples.screenshot-diff. Every state matches. A second copy-arm pass shows the same 0.20% at the first step, so that difference is capture noise. The states differ from each other (for example 4.5% between the first and fourth step), so a match means the screen was compared after it changed.adjustsFontSizeToFittext in a container of height 20 → 40 → 60 → 20 → 40directionRTL → LTR on the parentThe main thread didn't change. Perfetto:
NewGlobalRefsamples underParagraphShadowNode's clone constructor went from 201 to 0, and allNewGlobalRefsamples on the JS thread from 1.9% to 0.1% of the thread's samples.fixTextLayoutOnPixelDensityChange), thecom.facebook.react.views.textunit tests (this PR doesn't touch Java), Fantom (its text layout doesn't implement prepared layouts, so it doesn't reach this code), and an iOS build.🤖 Generated with Claude Code